-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create commerce context #8
base: main
Are you sure you want to change the base?
Conversation
src/CommerceContext.php
Outdated
* | ||
* @var array | ||
*/ | ||
protected $promotions = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about instead we just create an abstract base class, that cleans up something like "entities"?
src/CommerceContext.php
Outdated
* @Then I remove promotions :titles | ||
*/ | ||
public function iRemovePromotions($titles) { | ||
$promotions = \Drupal::entityTypeManager()->getStorage('commerce_promotion') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could move this into a base class as well, and also pass the entity type id as a parameter?
Regarding the failing tests, either there's a bug or I don't know the alphabet 😬 |
* Create coupon. | ||
*/ | ||
public function couponCreate($coupon) { | ||
$saved = $this->getDriver()->createEntity('commerce_promotion_coupon', $coupon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this make them automatically deleted after the scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'm pretty sure of that. I checked the db records for commerce_promotion_coupon
get cleared after a scenario with creating coupons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am now 100% sure that is not the case.
The entities are probably randomly deleted only because they are tied to a promotion deleted in the same cleanup. But there is nothing in our test classes or drupal extension to clean it up. We should do that here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be as easy as append it to $this->entities though
* Create coupon. | ||
*/ | ||
public function couponCreate($coupon) { | ||
$saved = $this->getDriver()->createEntity('commerce_promotion_coupon', $coupon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am now 100% sure that is not the case.
The entities are probably randomly deleted only because they are tied to a promotion deleted in the same cleanup. But there is nothing in our test classes or drupal extension to clean it up. We should do that here
* Create coupon. | ||
*/ | ||
public function couponCreate($coupon) { | ||
$saved = $this->getDriver()->createEntity('commerce_promotion_coupon', $coupon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be as easy as append it to $this->entities though
* @AfterScenario | ||
*/ | ||
public function cleanExchangeRates() { | ||
$database = \Drupal::database(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to either be wrapped in some conditional to check if the database table exists or wrapped in try/catch. This is not default commerce, and can be disabled on sites.
References
Changed
I wonder if it makes sense to break CommerceContext into even more, entity-based contexts like ProductContext, PromotionContext, OrderContext (I guess there might be one in the future), let me know.
Visuals
None
How can this be validated?
Pull request checklist